Alaa nasher w2 node.js #15
Conversation
Removed specific source directories and added keys.js to .gitignore.
RHSebregts
left a comment
RHSebregts
left a comment
There was a problem hiding this comment.
Hi Alaa,
Great work here! I left some remarks, but based on what I'm seeing here it seems like you have a good grasp on the basics of API's. Your test suite is also very good! There are some small changes you could make, to polish it even more.
Happy coding!
| app.post("/weather", async (req, res) => { | ||
| const { cityName } = req.body; | ||
| if (!cityName || cityName.trim() === "") { | ||
| return res.status(400).json({ msg: "City name is required." }); |
| try { | ||
| const response = await fetch(url); | ||
| if (!response.ok) { | ||
| return res.status(400).json({ weatherText: "City is not found!" }); |
There was a problem hiding this comment.
Since we're already in a try-catch we could also throw here.
| }); | ||
|
|
||
| describe("Test POST /weather", () => { | ||
| test("Return city name is required.", async () => { |
There was a problem hiding this comment.
I really like how this test is using the AAA principle. This makes it very readable, great!
| const res = await request.post("/weather").send({ cityName: city }); | ||
|
|
||
| expect(res.statusCode).toBe(200); | ||
| expect(res.body.main.temp).toBe(286.78); |
There was a problem hiding this comment.
We could also test for the city name here, to make sure we get all the correct data.
There was a problem hiding this comment.
This is a good test suite, well done! One small thing you could change is the order and grouping of the tests. You can put multiple tests into one describe block, which makes it easier to see which tests belong together, and cleans up your test logs a bit. It's also good to pick an order for tests, group them by end point and then either start with the happy path test and then error cases, or the other way around.
2 participants
No description provided.